Add jdbcAggregateOperationsRef to @EnableJdbcRepositories
#1704
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| * @param dataAccessStrategy must not be {@literal null}. | ||
| * @since 1.1 | ||
| */ | ||
| public JdbcAggregateTemplate(ApplicationContext publisher, RelationalMappingContext context, JdbcConverter converter, | ||
| public JdbcAggregateTemplate(ApplicationContext publisher, JdbcConverter converter, |
There was a problem hiding this comment.
Removed RelationalMappingContext because it can be obtained by JdbcConverter instance, but should it be kept for backward compatibility?
There was a problem hiding this comment.
I think it is better to create a new constructor here and mark the previous as deprecated in favor of new. We should not change public API for backward compatibility
| @@ -13,7 +13,7 @@ | |||
| * See the License for the specific language governing permissions and | |||
| * limitations under the License. | |||
| */ | |||
| package org.springframework.data.jdbc.repository.config; | |||
| package org.springframework.data.jdbc.dialect; | |||
There was a problem hiding this comment.
Moved package to avoid circular dependency between package repository.config and repository.support
There was a problem hiding this comment.
Agree, in r2dbc we already have it in dialect package
|
|
||
| } catch (NoSuchBeanDefinitionException exception) { | ||
|
|
||
| public JdbcCustomConversions jdbcCustomConversions(Optional<Dialect> dialect) { |
There was a problem hiding this comment.
Optional bean dependency can be expressed by Optional, which enables us to create JdbcCustomConversions beans easier for each data source (example).
There was a problem hiding this comment.
This change does not affect the feature and introduces the non-compatible change. Let's segregate it into separate PR so it can be merged in the next major release
There was a problem hiding this comment.
OK, I'll revert this change for now, and then create another PR that adds a factory class later.
kota65535
force-pushed
the
jdbc-aggregate-operations-ref
branch
from
bbe3881
to
aa2040d
Compare
| * @param dataAccessStrategy must not be {@literal null}. | ||
| * @since 1.1 | ||
| */ | ||
| public JdbcAggregateTemplate(ApplicationContext publisher, RelationalMappingContext context, JdbcConverter converter, | ||
| public JdbcAggregateTemplate(ApplicationContext publisher, JdbcConverter converter, |
There was a problem hiding this comment.
I think it is better to create a new constructor here and mark the previous as deprecated in favor of new. We should not change public API for backward compatibility
| @@ -13,7 +13,7 @@ | |||
| * See the License for the specific language governing permissions and | |||
| * limitations under the License. | |||
| */ | |||
| package org.springframework.data.jdbc.repository.config; | |||
| package org.springframework.data.jdbc.dialect; | |||
There was a problem hiding this comment.
Agree, in r2dbc we already have it in dialect package
|
|
||
| } catch (NoSuchBeanDefinitionException exception) { | ||
|
|
||
| public JdbcCustomConversions jdbcCustomConversions(Optional<Dialect> dialect) { |
There was a problem hiding this comment.
This change does not affect the feature and introduces the non-compatible change. Let's segregate it into separate PR so it can be merged in the next major release
|
@mikhail2048 Thanks for your review. I've fixed the points you mentined, please check it. |
|
@mp911de @schauder @mikhail2048 Could you please review when you have time? |
mp911de
added
type: enhancement
|
When can we hope to have this merged? It is quite nasty to use manual workarounds for multiple data sources. |
fix: #544, #687, #994
This PR adds
jdbcAggregateOperationsRefto@EnableJdbcRepositoriesthat enables to specify JdbcAggregateOperations bean name to be used for creating repositories, which significantly simplifies the configuration for multiple repositories.I've also created a sample project for testing this feature. Multiple repositories with different dialects can be configured using two configurations (Db1Config.java, Db2Config.java).
As you can see, creating the JdbcAggregateOperations bean is still a bit messy. It may be better to have some factory class for creating them.